[Emotion] Convert EuiDataGrid toolbar control styles to Emotion (Part 2) #7997

cee-chen · 2024-09-04T02:02:10Z

Summary

Part of #6394. I'm splitting up EuiDataGrid into smaller chunks for ease of review/QA, but as always, I still recommend following along by commit.

Converts the EuiDataGrid's various toolbar controls to Emotion. Also converts several Enzyme tests to RTL (largest set of changes is to column_selector.test.tsx - see git commit message)

QA

https://eui.elastic.co/pr_7997/#/tabular-content/data-grid-toolbar
Confirm no visual regressions against prod (background color is slightly different, this is an expected Emotion token change)
Open your browser devtools' responsive mode and reduce your screen size greatly. Confirm that tall popovers like the column selector and keyboard shortcuts shrink responsively with scrollbars
Optional Storybook QA

General checklist

Browser QA
- Checked in both light and dark modes
- Checked in mobile
- Checked in Chrome, Safari, Edge, and Firefox
  ~~- [ ] Checked for accessibility including keyboard-only and screenreader modes~~
Docs site QA - N/A
Code quality checklist
- Added or updated jest ~~and cypress~~ tests
- Updated visual regression tests
Release checklist
- A changelog entry exists and is marked appropriately.
  ~~- [ ] If applicable, added the breaking change issue label (and filled out the breaking change checklist)~~
Designer checklist - N/A

- convert margin left to display flex instead

- NOTE: possibly revert this commit when data grid borders modifiers are converted to Emotion - I'd rather handle this at the topmost `.euiDataGrid` level

- we're going to replace this with individual per-control Emotion styles

+ convert Enzyme tests to RTL + only test render snapshot on React 18 - all other tests run on react 16/17 and should more than suffice to validate that it works there + remove valid droppable area test - this one is ludicrous to try and test in a jsdom/unit test

+ reduce extra div wrapper + update datagrid enzyme tests to account for emotion wrappers

+ improve responsive styles by moving to `auto` column width

- just inline it in the component, since it's super minor + update Enzyme tests to RTL, snapshot full popover

elasticmachine · 2024-09-05T00:58:15Z

💚 Build Succeeded

Buildkite Build
Commit: 052965f

History

💚 Build #2689 succeeded 115c9f2
💔 Build #2680 failed 6ba298e

cee-chen · 2024-09-05T06:13:39Z

packages/eui/.storybook/test.ts

Just a quick note that 052965f / these VRT snapshots & utils technically won't work without #8003, which I decided to split it out into a separate PR (but left the generated screenshots in here for easier review).

mgadewoll

🚢 🐈‍⬛ I did not find any unexpected regression. The conversion code is straight forward and the added VRT stories and helpers are really awesome! 🚀

mgadewoll · 2024-09-05T07:57:23Z

packages/eui/src/components/datagrid/controls/data_grid_toolbar.styles.ts

@@ -22,6 +22,15 @@ export const euiDataGridToolbarStyles = ({ euiTheme }: UseEuiTheme) => {
      gap: ${euiTheme.size.base};
      ${logicalCSS('padding-vertical', euiTheme.size.xs)}
      background-color: ${euiTheme.colors.emptyShade};
+
+      .euiDataGrid--bordersHorizontal & {


❓ Do we need a TODO comment for keeping track of this to be reverted?

I'll be able to keep mental track of it! I'm not super worried either way, as the code works as-is, so even if I get hit by a bus tomorrow or we go with another direction for EuiDataGrid's Emotion conversion, this is fine to leave in the codebase.

mgadewoll · 2024-09-05T08:05:34Z

packages/eui/src/components/datagrid/controls/column_selector.test.tsx

    };

-    testByReactVersion(
+    testOnReactVersion(['18'])(


Agreed that we can save on snapshots for v16/17 as long as other tests still run 👍

mgadewoll · 2024-09-05T08:19:07Z

packages/eui/src/components/datagrid/controls/keyboard_shortcuts.tsx

  const keyboardShortcuts = useMemo(
    () => (
      <EuiPopover
        data-test-subj="dataGridKeyboardShortcutsPopover"
        isOpen={isOpen}
        closePopover={() => setIsOpen(false)}
        anchorPosition="downRight"
+        panelPaddingSize="none"


This change of padding placement caused a slight visual regression but I think it's a) a correct change and b) not a big visual impact.

Screen.Recording.2024-09-05.at.09.36.42.mov

Screen.Recording.2024-09-05.at.09.38.04.mov

Yep, it's an intentional change! 🎉 Thanks as always for your eagle eyes Lene!

mgadewoll · 2024-09-05T08:26:24Z

packages/eui/src/components/datagrid/controls/data_grid_toolbar.stories.tsx

+ * VRT tests only
+ */
+
+export const ColumnSelector: Story = {


🎉 Awesome update adding these VRT stories and additional helpers. Very useful! 👏

`v95.10.1`⏩`v95.11.0` > [!note] > The bulk of this release is **EuiDataGrid**. The component has been fully converted to Emotion, and several UX changes have been made to data cell actions. We recommend QA testing any data grid(s) that have custom styles applied to them. _[Questions? Please see our Kibana upgrade FAQ.](https://github.com/elastic/eui/blob/main/wiki/eui-team-processes/upgrading-kibana.md#faq-for-kibana-teams)_ --- ## [`v95.11.0`](https://github.com/elastic/eui/releases/v95.11.0) - Updated `EuiText`, `EuiTextColor`, and `EuiTextAlign` with a new `component` prop that allows changing the default rendered `<div>` wrapper to a `<span>` or `<p>` tag. ([#7993](elastic/eui#7993)) - Updated `EuiDataGrid`'s cell actions to always consistently be left-aligned, regardless of text content alignment ([#8011](elastic/eui#8011)) - Increased `EuiDataGrid`'s cell actions hover zone to reduce UX friction when mousing over from the grid cell to its actions ([#8011](elastic/eui#8011)) **Bug fixes** - Fixed `EuiPopover` to correctly inherit from `EuiProvider`'s `componentDefaults.EuiPortal.insert` ([#8003](elastic/eui#8003)) - Fixed push `EuiFlyoutResizable`s to not potentially block scrollbars on outside content ([#8010](elastic/eui#8010)) - Fixed an `EuiDataGrid` bug where the `setCellProps` callback passed by `renderCellValue` was not correctly applying custom `data-test-subj`s ([#8011](elastic/eui#8011)) **Accessibility** - Updated the `EuiBasicTable` actions button's `aria-label` by adding a reference to the current row ([#7994](elastic/eui#7994)) **CSS-in-JS conversions** - Converted `EuiDataGrid`'s toolbar controls to Emotion ([#7997](elastic/eui#7997)) - Removed `$euiDataGridPopoverMaxHeight` - Converted `EuiDataGrid` to Emotion ([#7998](elastic/eui#7998)) - Removed `$euiZDataGrid` - Removed `$euiZHeaderBelowDataGrid` - Converted `EuiDataGrid`'s `gridStyle`s to Emotion; Removed the following Sass variables and mixins: ([#8006](elastic/eui#8006)) - `$euiDataGridCellPaddingS` - `$euiDataGridCellPaddingM` - `$euiDataGridCellPaddingL` - `$euiDataGridVerticalBorder` - `$euiDataGridPrefix` - `$euiDataGridStyles` - `@euiDataGridSelector` - `@euiDataGridStyles` - Converted `EuiDataGrid`'s cell popover, actions, and focus outline to Emotion; Removed the following Sass variables and mixins: ([#8011](elastic/eui#8011)) - `$euiZDataGridCellPopover` - `@euiDataGridCellFocus` - Converted `EuiDataGrid`'s row, header, and footer cells to Emotion; Removed the following Sass variables and mixins: ([#8013](elastic/eui#8013)) - `$euiDataGridColumnResizerWidth` - `@euiDataGridRowCell` - `@euiDataGridHeaderCell` - `@euiDataGridFooterCell`

`v95.10.1`⏩`v95.11.0` > [!note] > The bulk of this release is **EuiDataGrid**. The component has been fully converted to Emotion, and several UX changes have been made to data cell actions. We recommend QA testing any data grid(s) that have custom styles applied to them. _[Questions? Please see our Kibana upgrade FAQ.](https://github.com/elastic/eui/blob/main/wiki/eui-team-processes/upgrading-kibana.md#faq-for-kibana-teams)_ --- ## [`v95.11.0`](https://github.com/elastic/eui/releases/v95.11.0) - Updated `EuiText`, `EuiTextColor`, and `EuiTextAlign` with a new `component` prop that allows changing the default rendered `<div>` wrapper to a `<span>` or `<p>` tag. ([elastic#7993](elastic/eui#7993)) - Updated `EuiDataGrid`'s cell actions to always consistently be left-aligned, regardless of text content alignment ([elastic#8011](elastic/eui#8011)) - Increased `EuiDataGrid`'s cell actions hover zone to reduce UX friction when mousing over from the grid cell to its actions ([elastic#8011](elastic/eui#8011)) **Bug fixes** - Fixed `EuiPopover` to correctly inherit from `EuiProvider`'s `componentDefaults.EuiPortal.insert` ([elastic#8003](elastic/eui#8003)) - Fixed push `EuiFlyoutResizable`s to not potentially block scrollbars on outside content ([elastic#8010](elastic/eui#8010)) - Fixed an `EuiDataGrid` bug where the `setCellProps` callback passed by `renderCellValue` was not correctly applying custom `data-test-subj`s ([elastic#8011](elastic/eui#8011)) **Accessibility** - Updated the `EuiBasicTable` actions button's `aria-label` by adding a reference to the current row ([elastic#7994](elastic/eui#7994)) **CSS-in-JS conversions** - Converted `EuiDataGrid`'s toolbar controls to Emotion ([elastic#7997](elastic/eui#7997)) - Removed `$euiDataGridPopoverMaxHeight` - Converted `EuiDataGrid` to Emotion ([elastic#7998](elastic/eui#7998)) - Removed `$euiZDataGrid` - Removed `$euiZHeaderBelowDataGrid` - Converted `EuiDataGrid`'s `gridStyle`s to Emotion; Removed the following Sass variables and mixins: ([elastic#8006](elastic/eui#8006)) - `$euiDataGridCellPaddingS` - `$euiDataGridCellPaddingM` - `$euiDataGridCellPaddingL` - `$euiDataGridVerticalBorder` - `$euiDataGridPrefix` - `$euiDataGridStyles` - `@euiDataGridSelector` - `@euiDataGridStyles` - Converted `EuiDataGrid`'s cell popover, actions, and focus outline to Emotion; Removed the following Sass variables and mixins: ([elastic#8011](elastic/eui#8011)) - `$euiZDataGridCellPopover` - `@euiDataGridCellFocus` - Converted `EuiDataGrid`'s row, header, and footer cells to Emotion; Removed the following Sass variables and mixins: ([elastic#8013](elastic/eui#8013)) - `$euiDataGridColumnResizerWidth` - `@euiDataGridRowCell` - `@euiDataGridHeaderCell` - `@euiDataGridFooterCell` (cherry picked from commit 9bbb296)

# Backport This will backport the following commits from `main` to `8.x`: - [Upgrade EUI to v95.11.0 (#192756)](#192756)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  --------- Co-authored-by: Cee Chen <[email protected]> Co-authored-by: Cee Chen <[email protected]>

cee-chen added the emotion label Sep 4, 2024

cee-chen added 9 commits September 4, 2024 10:27

Convert controls toolbar styles

09e19f8

- convert margin left to display flex instead

Handle grid borders style

8bd364b

- NOTE: possibly revert this commit when data grid borders modifiers are converted to Emotion - I'd rather handle this at the topmost `.euiDataGrid` level

[microperf] Make control button Emotion styles static

c5493fa

Delete .euiDataGrid__controlScroll CSS + Sass variable

d51373b

- we're going to replace this with individual per-control Emotion styles

Convert sorting control to Emotion

19633da

+ reduce extra div wrapper + update datagrid enzyme tests to account for emotion wrappers

Convert keyboard shortcuts to Emotion

0bdf395

+ improve responsive styles by moving to `auto` column width

Convert display selector width style

2e3b8d3

- just inline it in the component, since it's super minor + update Enzyme tests to RTL, snapshot full popover

Delete Sass files

6036bc1

cee-chen force-pushed the emotion/datagrid-3 branch from 6ba298e to 6036bc1 Compare September 4, 2024 18:57

changelog

115c9f2

cee-chen added the data grid label Sep 4, 2024

cee-chen changed the title ~~[EuiDataGrid] Convert toolbar styles to Emotion~~ [Emotion] Convert EuiDataGrid toolbar control styles to Emotion Sep 4, 2024

Add stories + VRT for datagrid toolbar

052965f

cee-chen marked this pull request as ready for review September 5, 2024 06:05

cee-chen requested a review from a team as a code owner September 5, 2024 06:05

cee-chen commented Sep 5, 2024

View reviewed changes

mgadewoll approved these changes Sep 5, 2024

View reviewed changes

cee-chen merged commit c60a91c into elastic:main Sep 5, 2024
6 checks passed

cee-chen deleted the emotion/datagrid-3 branch September 5, 2024 16:25

cee-chen changed the title ~~[Emotion] Convert EuiDataGrid toolbar control styles to Emotion~~ [Emotion] Convert EuiDataGrid toolbar control styles to Emotion (Part 2) Sep 5, 2024

cee-chen mentioned this pull request Sep 12, 2024

Upgrade EUI to v95.11.0 elastic/kibana#192756

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Emotion] Convert EuiDataGrid toolbar control styles to Emotion (Part 2) #7997

[Emotion] Convert EuiDataGrid toolbar control styles to Emotion (Part 2) #7997

cee-chen commented Sep 4, 2024 •

edited by mgadewoll

Loading

elasticmachine commented Sep 5, 2024

cee-chen Sep 5, 2024 •

edited

Loading

mgadewoll left a comment

mgadewoll Sep 5, 2024

cee-chen Sep 5, 2024 •

edited

Loading

mgadewoll Sep 5, 2024

mgadewoll Sep 5, 2024

cee-chen Sep 5, 2024

mgadewoll Sep 5, 2024

[Emotion] Convert EuiDataGrid toolbar control styles to Emotion (Part 2) #7997

[Emotion] Convert EuiDataGrid toolbar control styles to Emotion (Part 2) #7997

Conversation

cee-chen commented Sep 4, 2024 • edited by mgadewoll Loading

Summary

QA

General checklist

elasticmachine commented Sep 5, 2024

💚 Build Succeeded

History

cee-chen Sep 5, 2024 • edited Loading

Choose a reason for hiding this comment

mgadewoll left a comment

Choose a reason for hiding this comment

mgadewoll Sep 5, 2024

Choose a reason for hiding this comment

cee-chen Sep 5, 2024 • edited Loading

Choose a reason for hiding this comment

mgadewoll Sep 5, 2024

Choose a reason for hiding this comment

mgadewoll Sep 5, 2024

Choose a reason for hiding this comment

cee-chen Sep 5, 2024

Choose a reason for hiding this comment

mgadewoll Sep 5, 2024

Choose a reason for hiding this comment

cee-chen commented Sep 4, 2024 •

edited by mgadewoll

Loading

cee-chen Sep 5, 2024 •

edited

Loading

cee-chen Sep 5, 2024 •

edited

Loading